libelf: check loops for running away
authorIan Jackson <ian.jackson@eu.citrix.com>
Fri, 14 Jun 2013 15:39:36 +0000 (16:39 +0100)
committerIan Jackson <Ian.Jackson@eu.citrix.com>
Fri, 14 Jun 2013 15:39:36 +0000 (16:39 +0100)
commit39bf7b9d0ae534491745e54df5232127c0bddaf1
tree0808b27933081f9e463dd11041076a8d1784bd1a
parenta004800f8fc607b96527815c8e3beabcb455d8e0
libelf: check loops for running away

Ensure that libelf does not have any loops which can run away
indefinitely even if the input is bogus.  (Grepped for \bfor, \bwhile
and \bgoto in libelf and xc_dom_*loader*.c.)

Changes needed:
 * elf_note_next uses the note's unchecked alleged length, which might
   wrap round.  If it does, return ELF_MAX_PTRVAL (0xfff..fff) instead,
   which will be beyond the end of the section and so terminate the
   caller's loop.  Also check that the returned psuedopointer is sane.
 * In various loops over section and program headers, check that the
   calculated header pointer is still within the image, and quit the
   loop if it isn't.
 * Some fixed limits to avoid potentially O(image_size^2) loops:
    - maximum length of strings: 4K (longer ones ignored totally)
    - maximum total number of ELF notes: 65536 (any more are ignored)
 * Check that the total program contents (text, data) we copy or
   initialise doesn't exceed twice the output image area size.
 * Remove an entirely useless loop from elf_xen_parse (!)
 * Replace a nested search loop in in xc_dom_load_elf_symtab in
   xc_dom_elfloader.c by a precomputation of a bitmap of referenced
   symtabs.

We have not changed loops which might, in principle, iterate over the
whole image - even if they might do so one byte at a time with a
nontrivial access check function in the middle.

This is part of the fix to a security issue, XSA-55.

Signed-off-by: Ian Jackson <ian.jackson@eu.citrix.com>
Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>
v8: Fix the two loops in libelf-dominfo.c; the comment about
     PT_NOTE and SHT_NOTE wasn't true because the checks did
     "continue", not "break".
    Add a comment about elf_note_next's expectations of the caller's
     loop conditions (which most plausible callers will follow anyway).

v5: Fix regression due to wrong image size loop limit calculation.
    Check return value from xc_dom_malloc.

v4: Fix regression due to misplacement of test in elf_shdr_by_name
     (uninitialised variable).
    Introduce fixed limits.
    Avoid O(size^2) loops.
    Check returned psuedopointer from elf_note_next is correct.
    A few style fixes.

v3: Fix a whitespace error.

v2: BUGFIX: elf_shdr_by_name, elf_note_next: Reject new <= old, not just <.
    elf_shdr_by_name: Change order of checks to be a bit clearer.
    elf_load_bsdsyms: shdr loop check, improve chance of brokenness detection.
    Style fixes.
tools/libxc/xc_dom_elfloader.c
xen/common/libelf/libelf-dominfo.c
xen/common/libelf/libelf-loader.c
xen/common/libelf/libelf-tools.c
xen/include/xen/libelf.h